Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make allow attribute configurable in iframe panel #19087

Merged
merged 25 commits into from
Apr 15, 2024

Conversation

tomayac
Copy link
Contributor

@tomayac tomayac commented Dec 19, 2023

Proposed change

The iframe panel lets users embed arbitrary content. One use case is embedding widgets that interact with WebUSB (a concrete example is my Enphase Solar monitor that flashes a WebUSB-connected LED red or green based on the current power production). For this to work, the iframe needs to be granted USB access, which happens via the Permissions-Policy usb (<iframe allow="usb">.

I tried to start a discussion about this in #19028, but it's maybe too niche.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

cards:
  - type: iframe
    url: https://anchovy-honest-properly.ngrok-free.app/
    allow: usb

(I hope that this would work.)

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

home-assistant[bot]

This comment was marked as outdated.

@bramkragten
Copy link
Member

Your code does not match with your example, your example is for the dashboard iframe card but your code is for the iframe panel.

@tomayac
Copy link
Contributor Author

tomayac commented Jan 8, 2024

Thank you very much, @bramkragten! I have now added the same-ish logic to the iframe card that I had added to the iframe panel. I could not test this, so most likely it doesn't work yet. Is there someone familiar with the project who could verify this?

src/panels/iframe/ha-panel-iframe.ts Outdated Show resolved Hide resolved
src/panels/lovelace/cards/hui-iframe-card.ts Outdated Show resolved Hide resolved
src/panels/iframe/ha-panel-iframe.ts Outdated Show resolved Hide resolved
@tomayac tomayac requested a review from Quentame January 10, 2024 10:12
@@ -41,7 +41,7 @@ class HaPanelIframe extends LitElement {
)}
src=${this.panel.config.url}
sandbox="allow-forms allow-popups allow-pointer-lock allow-same-origin allow-scripts allow-modals allow-downloads"
allow="fullscreen"
allow=${this.panel.config.allow ?? 'fullscreen'}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires a backend change, please link to the core PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not sure what PR you're referring to. Could you clarify?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config of this panel is provided by core, so you will need to update core too for this to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I have tried my luck: home-assistant/core#108230. I very much admit that this is a bit out of my comfort zone.

@tomayac
Copy link
Contributor Author

tomayac commented Mar 11, 2024

Is #19087 jointly with home-assistant/core#108230 ready to be merged? Anything else needed from my end? @Quentame, you were involved in reviewing both. Thanks for your review again!

@tomayac
Copy link
Contributor Author

tomayac commented Apr 8, 2024

I have addressed the conflicts. Could you please take another look?

@bramkragten
Copy link
Member

The iframe panel is deprecated, we introduced the new webpage dashboard that will replace it. So can you please remove the code for the panel, then we can merge the code for the card.

@tomayac
Copy link
Contributor Author

tomayac commented Apr 12, 2024

Removed the iframe panel code. PTAL.

@bramkragten
Copy link
Member

Thanks! Please add a documentation PR for this feature

@tomayac
Copy link
Contributor Author

tomayac commented Apr 15, 2024

Thanks! Please add a documentation PR for this feature

Opened home-assistant/home-assistant.io#32321.

@bramkragten bramkragten enabled auto-merge (squash) April 15, 2024 11:16
@bramkragten bramkragten merged commit f21c89c into home-assistant:dev Apr 15, 2024
13 checks passed
@tomayac tomayac deleted the iframe-allow-attribute branch April 15, 2024 13:37
@habot-it
Copy link

Hello,

Would it be possible to add this option on iframe panel ?
Maybe here src/panels/iframe/ha-panel-iframe.ts --> line 45 ?

Thanks for your crazy work !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants